-
Notifications
You must be signed in to change notification settings - Fork 82
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add option to specify modifier in the worksheet API #492
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reset-object
modifier is specifically implemented for this feature https://scalameta.org/mdoc/docs/modifiers.html#reset-object
The object encoding can cause deadlocks and other difficult-to-debug errors while the class encoding at least fails fast with a recognizeable error message.
Metals can enable the reset-object
modifier without changing mdoc.
It seems we always add the Default modifier, so we would need to replace it with ResetObject one. What do you think? Or alternatively, add an option to specify the modifier to the worksheet API. |
Right, it looks like worksheet clients can't add modifiers. It should be an easy change to make this configurable, a good place to look at would be here
There's another overload for |
9659667
to
4f56125
Compare
4f56125
to
c7958d5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
| case class Bar(b: Int) extends AnyVal | ||
|}""".stripMargin, | ||
"", | ||
modifier = Some("reset-object") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to add multiple modifiers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is! Added a test to demonstrate it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome
c7958d5
to
ce9e47d
Compare
Modifier doesn't exists for Scala 3 currently, so we should be fine to wait for #466 |
5056dcb
to
c6807f1
Compare
c6807f1
to
61c5c5c
Compare
Related to scalameta/metals#2764